contains operator #99
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ytes to compare with bytes_lit
thearossman
reviewed
Feb 19, 2025
filtergen/src/lib.rs
Outdated
| //! | `<` | `lt` | Less than | `ipv6.payload_length < 500` | | ||
| //! | `in` | | In a range, or in a subnet | `ipv4.src_addr in 1.2.3.4/16` | | ||
| //! | `~` | `matches` | Regular expression match | `tls.sni ~ 'netflix\\.com$'` | | ||
| //! | Operator | Alias | Description | Example | |
There was a problem hiding this comment.
Not a huge deal, but do you know what changed here to cause this diff? Seems like there shouldn't actually be a change here.
thearossman
reviewed
Feb 19, 2025
filtergen/src/lib.rs
Outdated
| //! | `<` | `lt` | Less than | `ipv6.payload_length < 500` | | ||
| //! | `in` | | In a range, or in a subnet | `ipv4.src_addr in 1.2.3.4/16` | | ||
| //! | `~` | `matches` | Regular expression match | `tls.sni ~ 'netflix\\.com$'` | | ||
| //! | `contains` | | Check if right appears in left | `tls.sni contains \|2E 63 6F 6D\|` | |
There was a problem hiding this comment.
Is the \ required to indicate bytes?
thearossman
reviewed
Feb 19, 2025
filtergen/src/utils.rs
Outdated
| let bytes_lit = syn::LitByteStr::new(b, Span::call_site()); | ||
| let num_bytes = bytes_lit.value().len(); | ||
| quote! { | ||
| #proto.#field().as_bytes().windows(#num_bytes).any(|w| w == #bytes_lit) |
There was a problem hiding this comment.
I know that we talked about this and I said windows seemed like a good idea, but I just found the following crates that look like they would be more efficient. I think these would work for both strings and bytes.
- https://docs.rs/twoway/latest/twoway/ -- looks like two-way string matching will be a lot faster for longer strings in particular, though I'm not sure what "longer" means.
- https://docs.rs/memchr/latest/memchr/memmem/fn.find.html -- I'm not sure if this requires specialized HW support to actually be faster though? Based on the documentation, it looks like initializing one
Finderand adding it to statics could also be faster.
Could you do a bit of research into these and figure out which one makes the most sense? I don't know that we have to go into depth profiling them, but worth taking a look.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR implements support for a
containsoperator that can be used with strings or bytes. When used with bytes, the protocol field of the filter can be a string or a byte (if it's a string, it gets converted to a byte before comparison is done).